Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine completion script tests #699

Merged
merged 1 commit into from
Feb 6, 2025
Merged

Combine completion script tests #699

merged 1 commit into from
Feb 6, 2025

Conversation

rauhul
Copy link
Contributor

@rauhul rauhul commented Feb 6, 2025

Removes a ton of redundant completion script tests and combines unique components into a single common set of tests.

Removes a ton of redundant completion script tests and combines unique
components into a single common set of tests.
@rauhul rauhul requested a review from natecook1000 February 6, 2025 17:41
@rauhul
Copy link
Contributor Author

rauhul commented Feb 6, 2025

@swift-ci test

@rauhul
Copy link
Contributor Author

rauhul commented Feb 6, 2025

@rgoldberg heads up

@rauhul rauhul enabled auto-merge (squash) February 6, 2025 17:43
@rgoldberg
Copy link
Contributor

@rauhul @natecook1000 Looks like you guys might be getting ready to release a new version.

Any chance of discussing the issue mentioned in the comments of #679?

Thanks for the heads up about this change.

@shahmishal
Copy link
Member

@swift-ci test macOS

@rauhul
Copy link
Contributor Author

rauhul commented Feb 6, 2025

@rgoldberg We're happy to take PRs to improve these things, I dont know nearly enough about the completion stuff to have many opinions here

@rauhul
Copy link
Contributor Author

rauhul commented Feb 6, 2025

@rgoldberg My goal is to move all these generators to using ToolInfo instead of the raw internals of argument parser. ToolInfo is basically our easier to use IR for the command structure.

@rauhul rauhul merged commit 45b5c74 into main Feb 6, 2025
2 checks passed
@rauhul rauhul deleted the completion-snapshot-clean branch February 6, 2025 17:50
@rgoldberg
Copy link
Contributor

@rauhul thanks for the info.

I've already made many changes in my local branch using the ArgumentParser internals. I know nothing about ToolInfo. Would SAP accept numerous fixes using the internals of ArgumentParser? Maybe I could switch to ToolInfo in a subsequent PR…

Is there anyone else besides @natecook1000 with whom I should discuss #679?

@rauhul
Copy link
Contributor Author

rauhul commented Feb 6, 2025

Lets continue this discussion on #695.

We'd love improvements in this area. Its important for evolution to decouple all these generators from the internals to a more stable API (ToolInfo)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants